Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

coreclr target for FSharp.LanguageService.Compiler #1062

Closed
wants to merge 2 commits into from
Closed

coreclr target for FSharp.LanguageService.Compiler #1062

wants to merge 2 commits into from

Conversation

ncave
Copy link
Contributor

@ncave ncave commented Apr 12, 2016

Enabled coreclr target for FSharp.LanguageService.Compiler, and removed a few internal modifiers here and there to support Fable etc.

@msftclas
Copy link

Hi @ncave, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

In order for us to evaluate and accept your PR, we ask that you sign a contribution license agreement. It's all electronic and will take just minutes. I promise there's no faxing. https://cla.microsoft.com.

TTYL, MSBOT;

@msftclas
Copy link

@ncave, Thanks for signing the contribution license agreement so quickly! Actual humans will now validate the agreement and then evaluate the PR.

Thanks, MSBOT;

@dsyme
Copy link
Contributor

dsyme commented Apr 12, 2016

@ncave FSharp.LanguageService.Compiler is a cut-down version of FSharp.Compiler.Service which is private to the Visual F# Tools. It is not meant to have a public API.

Could you instead submit this to http://github.com/fsharp/FSharp.Compiler.Service as part of a larger PR to enable a coreclr build of that component?

(It is possible we should move all FSharp.Compiler.Service development to this repo but we're not there yet, partly because there's a bunch of functionality in FCS that's not here, like the Expressions API, and partly to avoid this repo becoming a logjam).

@dsyme
Copy link
Contributor

dsyme commented Apr 12, 2016

I'll close this since we can't remove the "internal" identifiers from this particular component.

@dsyme dsyme closed this Apr 12, 2016
@7sharp9
Copy link
Contributor

7sharp9 commented Apr 12, 2016

Could this be added to FCS instead perhaps?

@ncave
Copy link
Contributor Author

ncave commented Apr 12, 2016

@dsyme: Thank you for responding so quickly, can you please clarify what is the best way to maintain the three repositories in sync, as the language evolves? Should FCS be merged into fsharp/fsharp, can we make a coreclr branch in fsharp/fsharp that has FCS as well? What is the flow between fsharp/fsharp and ms/visualfsharp when not in sync?
It does make sense for the compiler service to be in the same repository with the compiler since they share so much code, so there will be less code to maintain.
The internal modifiers can be handled with defines to only expose modules in the compiler service, not in the compiler itself, if that's a requirement.

@7sharp9
Copy link
Contributor

7sharp9 commented Apr 12, 2016

I single repo is really needed, it makes working on compiler features a massive pain, not one I enjoy. Maintaining three repos to test and develop is not good.

@dsyme
Copy link
Contributor

dsyme commented Apr 12, 2016

@ncave The FCS issue to discuss CoreCLR support is here: fsharp/fsharp-compiler-docs#465. It's been dormant for a while but it's probably time to bring it to life again.

The relationship between repositories is explained here:http://fsharp.github.io/2014/06/18/fsharp-contributions.html.

Basically:

  • We regularly merge from Microsoft/visualfsharp --> fsharp/fsharp --> fsharp/FSharp.Compiler.Service.
  • We reverse integrate some functionality from fsharp/FSharp.Compiler.Service. to Microsoft/visualfsharp opportunistically
  • However some more nascent FCS functionality (like the typed expression trees that Fable uses) has not been brought across. In effect the FCS repo is a proving ground for FCS features which is sufficiently high quality (or fast-to-fix) for the F# community tools such as Visual F# Power Tools, but which needs a lot of proofing in this repo.

@7sharp9 It's work. It's also culture change at Microsoft (e.g. do we have the energy to support our repos being compiled into Mono editions of things? Are we willing for community people to have the right to accept pull requests into this repo? )

We're gradually getting there. But honestly, don't expect that it would make things "easy". if we merged all FCS and F# Mono work into this repo then it would become a real logjam: and build, test, bug tracking and CI would become harder.

Of course, one repo would mean no integration. But you might find that even mid-quality FCS PRs are just not accepted for fear of destabilization. At Microsoft it only takes one bad bug in a shipping RTM or Update of Visual Studio to make people very cautious.

TBH I don't actually find it that hard to work on compiler features here, as it lets me focus on Windows and CoreCLR support at Microsoft quality levels. I think it is, however, very hard to do language/compiler work on a non-Windows machine (which I think is what you do). That is changing with CoreCLR support in this repo.

@ncave
Copy link
Contributor Author

ncave commented Apr 13, 2016

Thank you for the extensive response, I'll wait for fsharp/fsharp#558.

@ncave ncave deleted the coreclr branch April 13, 2016 09:10
@dsyme
Copy link
Contributor

dsyme commented Apr 13, 2016

@ncave Let's follow up on fsharp/fsharp-compiler-docs#465

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants